Conversation
|
Just saw that there is also a master branch, but couldn't spot the differences. Rebasing shouldn't be problem when develop is wrong. |
|
Thank you for the fix, @Vollstrecker Your approach looks correct, but it must maintain compatibility with the older zlib versions that we still support. If you can adjust the patch so it works across all zlib versions from 1.2.11 onwards, that'd be greatly appreciated. On the commit layout: can you please squash all commits into a single one and force-push that to your branch in your repo? The pull request should have one commit, unless there's a specific reason to have more. The libpng history is quite dense already, and we're aiming to limit one commit per resolved issue going forward. |
including generation of PNGConfig.cmake and support for finding PNG by components
Vollstrecker
force-pushed
the
develop
branch
from
8ed86c4 to
31f052e
Compare
|
Should work now. I can't test here with older zlib, But I'm on Debian which doesn't ship a config at all, so it has to fall back to old style. I had problems with libpngconf, as they are not regenerated when switching between the zlib-versions, but I guess that's a common problem but not a common use-case. |
Bug in zlib. Its CMake config should not try to include files it didn't produce. IMHO libpng should not do anything special beyond calling |
As you may have seen this was relaxed in the fixes for the next release, and no it's not a bug.
Works if both variants are there, fixes only prints a warning. If you want static, specify that, where's the problem?
IMHO this is a bug when ZLIB::ZLIB point to the shard lib, as any user won't know to also link zlib. And using only ZLIB::ZLIB would mean that pngstatic needs also a shared lib. Static libs should link static and shared libs should link shared, and static should link static - not possible with one target name for both. |
3 participants
Hi,
as you may have noticed the new release of zlib changed their config, so if not all (as in shared and static) libs are found it fails if you not specify that you only want to use the static lib.
As an example a user told me that libpng fails searching for zlib when he only builds one of the libs. I don't know how he configured zlib and png, as libpng only used find_package(ZLIB REQUIRED) the config should be a fallback, maybe he meant when finding libpng via config, then I guess the CONFIG flag will also be used by find_dependency.
So I took the oportunity to kickstart the migration here. With this patch libpng uses the provided config if found and the old behaviour as a fallback. I kept the behaviour of switching to the shared zlib if the static one wasn't found, I consider this wrong, as a static lib shouldn't depend on shared ones, but it is how this project here was designed, so I'll change this only when explicit requested.
When importing PNGConfig.cmake it will use ZLIBConfig.cmake with the needed COMPONENTS, as I'm pretty sure noone builds zlib and png just to transfer png to a different mashine but building zlib there with different options. When png was built without ZLIBConfig, nothing changes.
While doing this I also added support for searching only for shared/static png by using find_package(PNG CONFIG COMPONENTS shared static) or only one of them. Contrary to zlib's config I only error out when someone requests components and they are not found, when no components are given, nothing changes. Again, I like the zlib approach better and would make these changes if asked for.